[WIP] feat: render chinese characters via cloudinary.#32
[WIP] feat: render chinese characters via cloudinary.#32thorwebdev wants to merge 4 commits intobadger:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for rendering Chinese characters in GitHub usernames by using Cloudinary's text rendering service as an image when the device's native font doesn't support Chinese characters.
Key changes:
- Added Cloudinary cloud name configuration to secrets.py with a default "demo" value
- Implemented Chinese character detection and image-based rendering for names containing Chinese characters
- Modified the badge display logic to use image rendering for Chinese names and fallback to text for non-Chinese names
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| badge/secrets.py | Added CLOUDINARY_CLOUD_NAME configuration with documentation |
| badge/apps/badge/init.py | Added Chinese character detection, Cloudinary URL generation, image fetching, and conditional rendering logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not self._task: | ||
| self._task = get_user_data(self, self._force_update) |
There was a problem hiding this comment.
This condition calls get_user_data() again which will re-fetch user data unnecessarily. The Chinese image should already be fetched within get_user_data() (lines 182-192). This condition appears to be unreachable in normal flow since chinese_name_img is set during the initial get_user_data() call when self.name is populated.
| if not self._task: | |
| self._task = get_user_data(self, self._force_update) |
| if self.chinese_name_img: | ||
| # Display Chinese name as image, centered | ||
| img_w = self.chinese_name_img.width | ||
| screen.blit(self.chinese_name_img, 80 - (img_w / 2), 16) |
There was a problem hiding this comment.
Integer division should be used for pixel positioning. Using float division may cause rendering issues. Change to 80 - (img_w // 2) for consistent behavior with the text centering logic on line 369.
| screen.blit(self.chinese_name_img, 80 - (img_w / 2), 16) | |
| screen.blit(self.chinese_name_img, 80 - (img_w // 2), 16) |
| if self.chinese_name_img: | ||
| # Display Chinese name as image, centered | ||
| img_w = self.chinese_name_img.width | ||
| screen.blit(self.chinese_name_img, 80 - (img_w / 2), 16) |
There was a problem hiding this comment.
[nitpick] The Y-coordinate 16 is hardcoded here and on line 369. Consider extracting this as a constant like NAME_Y_POSITION = 16 to improve maintainability and ensure consistency.
|
I love this approach, although I'd hesitate to publish usernames to a 3rd party service without updating appropriate privacy statements etc. Might see if it's possible to do something similar but with a GitHub hosted service or something on GitHub Pages. Alternatively investigate full unicode font support and how compressed we can make those font files. |
|
@martinwoodward ah yah, good point on privacy. Any thoughts on a better approach? The chinese character alphabet is just so large it wouldn't fit on the device I think :( |
2 participants
fixes #29